fix(rosetta): infused snippets not returned from cache #3291
Conversation
| @@ -149,6 +149,13 @@ export class RosettaTranslator { | |||
| function tryReadFromCache(sourceSnippet: TypeScriptSnippet, cache: LanguageTablet, fingerprinter: TypeFingerprinter) { | |||
| const fromCache = cache.tryGetSnippet(snippetKey(sourceSnippet)); | |||
|
|
|||
| // infused snippets won't pass the full source check or the fingerprinter | |||
There was a problem hiding this comment.
Why do they not pass the fingerprinter? I tried to investigate this, and I can't really make sense of it.
Verified that the fqns are the same between fromCache.fqnsReferenced() and fromCache.snippet.fqnsReferenced. The only difference is that locally I removed the monocdk assembly and the fingerprinter takes in all assemblies at initialization. But I fail to see how that impacts anything here.
A bit of a cop out, but I think its unnecessary. We cant translate infused examples, so we might as well take what we've got. Thoughts?
There was a problem hiding this comment.
I'm surprised as well, but I agree with the solution. It's fine like this.
| @@ -149,6 +149,13 @@ export class RosettaTranslator { | |||
| function tryReadFromCache(sourceSnippet: TypeScriptSnippet, cache: LanguageTablet, fingerprinter: TypeFingerprinter) { | |||
| const fromCache = cache.tryGetSnippet(snippetKey(sourceSnippet)); | |||
|
|
|||
| // infused snippets won't pass the full source check or the fingerprinter | |||
There was a problem hiding this comment.
I'm surprised as well, but I agree with the solution. It's fine like this.
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
Infused snippets do not have the right full source since we are adding the local fixture to the snippet
rather than the fixture from where the snippet came from. Since there is no hope in actually translating
an infused snippet (unless it is in the same directory), we might as well take any cache result as is.
So this PR adds logic that returns the cache result for infused snippets always.
This was tested on my local machine and dropped all errors from infused snippets. There are still errors
from directories that are not yet strict, which is understandable.
Also, the infuse command was missing
cache-fromargument that is present ininfuseOptions.We were using those options as part of
rosetta extract --infusebut should be available viarosetta infuseas well.
Also fixed up minor mistake in tests where I was testing for
infused=trueinstead ofinfused=''.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.